-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Introduce a "log level" support for DEBUG_TYPE #150855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-llvm-support Author: Mehdi Amini (joker-eph) ChangesThis allows to set an optional integer level for a given debug type. The string format is
This means that when checking which level is enabled, 0 and 1 are identical. The LDBG() macro is updated to accept an optional log level to illustrate the feature. Here is the expected behavior: LDBG() << "A"; With Note that LDBG() is equivalent to LDBG(0) which is equivalent to LDBG(1). Full diff: https://github.com/llvm/llvm-project/pull/150855.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Support/Debug.h b/llvm/include/llvm/Support/Debug.h
index 924d7b216438e..2ce0bda3bdbbf 100644
--- a/llvm/include/llvm/Support/Debug.h
+++ b/llvm/include/llvm/Support/Debug.h
@@ -39,13 +39,18 @@ class raw_ostream;
/// isCurrentDebugType - Return true if the specified string is the debug type
/// specified on the command line, or if none was specified on the command line
/// with the -debug-only=X option.
-///
-bool isCurrentDebugType(const char *Type);
+/// An optional level can be provided to control the verbosity of the output.
+/// If the provided level is not 0 and user specified a level below the provided
+/// level, the output is disabled.
+bool isCurrentDebugType(const char *Type, int Level = 0);
/// setCurrentDebugType - Set the current debug type, as if the -debug-only=X
/// option were specified. Note that DebugFlag also needs to be set to true for
/// debug output to be produced.
-///
+/// The debug type format is "type[:level]", where the level is an optional
+/// integer. The default level is 0, which is the most verbose.
+/// The level can be set to 1, 2, 3, etc. to control the verbosity of the
+/// output. The level can be set to -1 to disable the output.
void setCurrentDebugType(const char *Type);
/// setCurrentDebugTypes - Set the current debug type, as if the
diff --git a/llvm/include/llvm/Support/DebugLog.h b/llvm/include/llvm/Support/DebugLog.h
index 19d309865bbd4..2541d60535ff7 100644
--- a/llvm/include/llvm/Support/DebugLog.h
+++ b/llvm/include/llvm/Support/DebugLog.h
@@ -19,29 +19,57 @@
namespace llvm {
#ifndef NDEBUG
-// Output with given inputs and trailing newline. E.g.,
+// LDBG() is a macro that can be used as a raw_ostream for debugging.
+// It will stream the output to the dbgs() stream, with a prefix of the
+// debug type and the file and line number. A trailing newline is added to the
+// output automatically. If the streamed content contains a newline, the prefix
+// is added to each beginning of a new line. Nothing is printed if the debug
+// output is not enabled or the debug type does not match.
+//
+// An optional `level` argument can be provided to control the verbosity of the
+// output. The default level is 0, which is the most verbose. The level can be
+// set to 1, 2, 3, etc. to control the verbosity of the output.
+//
+// The `level` argument can be a literal integer, or a macro that evaluates to
+// an integer.
+//
+// E.g.,
// LDBG() << "Bitset contains: " << Bitset;
// is equivalent to
-// LLVM_DEBUG(dbgs() << DEBUG_TYPE << " [" << __FILE__ << ":" << __LINE__
-// << "] " << "Bitset contains: " << Bitset << "\n");
-#define LDBG() DEBUGLOG_WITH_STREAM_AND_TYPE(llvm::dbgs(), DEBUG_TYPE)
+// LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] " << __FILE__ << ":" <<
+// __LINE__ << " "
+// << "Bitset contains: " << Bitset << "\n");
-#define DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, TYPE, FILE) \
- for (bool _c = (::llvm::DebugFlag && ::llvm::isCurrentDebugType(TYPE)); _c; \
- _c = false) \
+#define _GET_LDBG_MACRO(_1, NAME, ...) NAME
+#define LDBG(...) \
+ _GET_LDBG_MACRO(__VA_ARGS__, LDBG_LOG_LEVEL, LDBG_LOG_LEVEL_0)(__VA_ARGS__)
+
+#define LDBG_LOG_LEVEL(LEVEL) \
+ DEBUGLOG_WITH_STREAM_AND_TYPE(llvm::dbgs(), LEVEL, DEBUG_TYPE)
+#define LDBG_LOG_LEVEL_0() LDBG_LOG_LEVEL(0)
+
+#define DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, \
+ LINE) \
+ for (bool _c = \
+ (::llvm::DebugFlag && ::llvm::isCurrentDebugType(TYPE, LEVEL)); \
+ _c; _c = false) \
::llvm::impl::raw_ldbg_ostream{ \
- ::llvm::impl::computePrefix(TYPE, FILE, __LINE__), (STREAM)} \
+ ::llvm::impl::computePrefix(TYPE, FILE, LINE), (STREAM)} \
.asLvalue()
+
+#define DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, LEVEL, TYPE, FILE) \
+ DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(STREAM, LEVEL, TYPE, FILE, __LINE__)
// When __SHORT_FILE__ is not defined, the File is the full path,
// otherwise __SHORT_FILE__ is defined in CMake to provide the file name
// without the path prefix.
#if defined(__SHORT_FILE__)
-#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, TYPE) \
- DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, TYPE, __SHORT_FILE__)
+#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, LEVEL, TYPE) \
+ DEBUGLOG_WITH_STREAM_TYPE_AND_FILE(STREAM, LEVEL, TYPE, __SHORT_FILE__)
#else
-#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, TYPE) \
+#define DEBUGLOG_WITH_STREAM_AND_TYPE(STREAM, LEVEL, TYPE) \
DEBUGLOG_WITH_STREAM_TYPE_AND_FILE( \
- STREAM, TYPE, ::llvm::impl::LogWithNewline::getShortFileName(__FILE__))
+ STREAM, LEVEL, TYPE, \
+ ::llvm::impl::LogWithNewline::getShortFileName(__FILE__))
#endif
namespace impl {
diff --git a/llvm/lib/Support/Debug.cpp b/llvm/lib/Support/Debug.cpp
index 5bb04d0c22998..1ad758ae9471a 100644
--- a/llvm/lib/Support/Debug.cpp
+++ b/llvm/lib/Support/Debug.cpp
@@ -24,11 +24,13 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/Debug.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/circular_raw_ostream.h"
#include "llvm/Support/raw_ostream.h"
+#include <utility>
#include "DebugOptions.h"
@@ -38,25 +40,46 @@
using namespace llvm;
+/// Parse a debug type string into a pair of the debug type and the debug level.
+/// The expected format is "type[:level]", where the level is an optional
+/// integer.
+static std::pair<std::string, int> parseDebugType(StringRef DbgType) {
+ auto [Type, LevelStr] = DbgType.split(':');
+ int Level;
+ if (!to_integer(LevelStr, Level, 10))
+ Level = 0;
+ return std::make_pair(Type.str(), Level);
+}
+
// Even though LLVM might be built with NDEBUG, define symbols that the code
// built without NDEBUG can depend on via the llvm/Support/Debug.h header.
namespace llvm {
/// Exported boolean set by the -debug option.
bool DebugFlag = false;
-static ManagedStatic<std::vector<std::string>> CurrentDebugType;
+/// The current debug type and an optional debug level.
+/// The debug level is the verbosity of the debug output.
+/// The default level is 0, which is the most verbose.
+/// The level can be set to 1, 2, 3, etc. to control the verbosity of the
+/// output. The level can be set to -1 to disable the output.
+static ManagedStatic<std::vector<std::pair<std::string, int>>> CurrentDebugType;
/// Return true if the specified string is the debug type
/// specified on the command line, or if none was specified on the command line
/// with the -debug-only=X option.
-bool isCurrentDebugType(const char *DebugType) {
+bool isCurrentDebugType(const char *DebugType, int Level) {
if (CurrentDebugType->empty())
return true;
// See if DebugType is in list. Note: do not use find() as that forces us to
// unnecessarily create an std::string instance.
for (auto &d : *CurrentDebugType) {
- if (d == DebugType)
- return true;
+ if (d.first == DebugType) {
+ if (d.second < 0)
+ return false;
+ if (d.second == 0)
+ return true;
+ return d.second >= Level;
+ }
}
return false;
}
@@ -73,8 +96,11 @@ void setCurrentDebugType(const char *Type) {
void setCurrentDebugTypes(const char **Types, unsigned Count) {
CurrentDebugType->clear();
- llvm::append_range(*CurrentDebugType, ArrayRef(Types, Count));
+ CurrentDebugType->reserve(Count);
+ for (const char *Type : ArrayRef(Types, Count))
+ CurrentDebugType->push_back(parseDebugType(Type));
}
+
} // namespace llvm
// All Debug.h functionality is a no-op in NDEBUG mode.
@@ -114,10 +140,10 @@ struct DebugOnlyOpt {
if (Val.empty())
return;
DebugFlag = true;
- SmallVector<StringRef,8> dbgTypes;
- StringRef(Val).split(dbgTypes, ',', -1, false);
- for (auto dbgType : dbgTypes)
- CurrentDebugType->push_back(std::string(dbgType));
+ SmallVector<StringRef, 8> DbgTypes;
+ StringRef(Val).split(DbgTypes, ',', -1, false);
+ for (auto DbgType : DbgTypes)
+ CurrentDebugType->push_back(parseDebugType(DbgType));
}
};
} // namespace
@@ -130,7 +156,11 @@ struct CreateDebugOnly {
return new cl::opt<DebugOnlyOpt, true, cl::parser<std::string>>(
"debug-only",
cl::desc("Enable a specific type of debug output (comma separated list "
- "of types)"),
+ "of types using the format \"type[:level]\", where the level "
+ "is an optional integer. The default level is 0, which is the "
+ "most verbose. The level can be set to 1, 2, 3, etc. to "
+ "control the verbosity of the output. The level can be set to "
+ "-1 to disable the output."),
cl::Hidden, cl::value_desc("debug string"),
cl::location(DebugOnlyOptLoc), cl::ValueRequired);
}
diff --git a/llvm/unittests/Support/DebugLogTest.cpp b/llvm/unittests/Support/DebugLogTest.cpp
index c34d888ab4cad..1886c3ef59cfc 100644
--- a/llvm/unittests/Support/DebugLogTest.cpp
+++ b/llvm/unittests/Support/DebugLogTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/DebugLog.h"
+#include "llvm/ADT/Sequence.h"
#include "llvm/Support/raw_ostream.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -26,7 +27,7 @@ TEST(DebugLogTest, Basic) {
{
std::string str;
raw_string_ostream os(str);
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, nullptr) << "NoType";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, nullptr) << "NoType";
EXPECT_FALSE(StringRef(os.str()).starts_with('['));
EXPECT_TRUE(StringRef(os.str()).ends_with("NoType\n"));
}
@@ -35,8 +36,8 @@ TEST(DebugLogTest, Basic) {
{
std::string str;
raw_string_ostream os(str);
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "A") << "A";
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "B") << "B";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "A") << "A";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "B") << "B";
EXPECT_TRUE(StringRef(os.str()).starts_with('['));
EXPECT_THAT(os.str(), AllOf(HasSubstr("A\n"), HasSubstr("B\n")));
}
@@ -47,22 +48,39 @@ TEST(DebugLogTest, Basic) {
raw_string_ostream os(str);
// Just check that the macro doesn't result in dangling else.
if (true)
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "A") << "A";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "A") << "A";
else
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "A") << "B";
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "B") << "B";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "A") << "B";
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "B") << "B";
EXPECT_THAT(os.str(), AllOf(HasSubstr("A\n"), Not(HasSubstr("B\n"))));
int count = 0;
auto inc = [&]() { return ++count; };
EXPECT_THAT(count, Eq(0));
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "A") << inc();
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "A") << inc();
EXPECT_THAT(count, Eq(1));
- DEBUGLOG_WITH_STREAM_AND_TYPE(os, "B") << inc();
+ DEBUGLOG_WITH_STREAM_AND_TYPE(os, 0, "B") << inc();
EXPECT_THAT(count, Eq(1));
}
}
+TEST(DebugLogTest, BasicWithLevel) {
+ llvm::DebugFlag = true;
+ // We expect A to be always printed, B to be printed only when level is 1 or
+ // below, and C to be never printed.
+ static const char *DT[] = {"A:0", "B:1", "C:-1"};
+
+ setCurrentDebugTypes(DT, 3);
+ std::string str;
+ raw_string_ostream os(str);
+ for (auto type : {"A", "B", "C"})
+ for (int level : llvm::seq<int>(0, 3))
+ DEBUGLOG_WITH_STREAM_TYPE_FILE_AND_LINE(os, level, type, type, level)
+ << level;
+ EXPECT_EQ(os.str(),
+ "[A] A:0 0\n[A] A:1 1\n[A] A:2 2\n[B] B:0 0\n[B] B:1 1\n");
+}
+
TEST(DebugLogTest, StreamPrefix) {
llvm::DebugFlag = true;
static const char *DT[] = {"A", "B"};
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces log level support for the LLVM DEBUG_TYPE system, allowing users to control debug output verbosity by specifying optional integer levels with debug types. The format is type[:level] where level 0 (default) enables all debug output, -1 disables it, and positive values enable output up to that verbosity level.
Key changes:
- Enhanced debug type parsing to support optional log levels with format
type[:level] - Updated LDBG() macro to accept optional level parameter for controlling output verbosity
- Modified core debug infrastructure to track and evaluate debug levels when determining output eligibility
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| llvm/lib/Support/Debug.cpp | Core implementation of debug level parsing and filtering logic |
| llvm/include/llvm/Support/Debug.h | Updated API signatures to support debug levels |
| llvm/include/llvm/Support/DebugLog.h | Enhanced LDBG macro with level support and updated prefix formatting |
| llvm/unittests/Support/DebugLogTest.cpp | Added comprehensive test coverage for new level functionality |
ed76f4d to
ae03344
Compare
jpienaar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. In another lib there was 0 == always log, 1 == log if enabled, so here it could have been to enable even if debug type is not set (e.g., if 0 then just emit rather than checking if debugging enabled). That was rather convenient during debugging (especially for cases where the pass is so deeply nested that plumbing flag through is tricky) where if needed, I could set all to 0 in the file.
llvm/include/llvm/Support/DebugLog.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
llvm/lib/Support/Debug.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm, so 'a' would enable but empty disables? E.g., why not just disable by not listing and treating any error as level 0/1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is providing a "negative filter": that is instead of opting out for specific DEBUG_TYPE you can opt out.
For example running with:
--debug-only=pattern-application:-1
Will print everything but the pattern-application type.
9271669 to
0426a5c
Compare
This allows to set an optional integer level for a given debug type. The string format is `type[:level]`, and the integer is interpreted as such: - if 0 (default): all debugging for this debug type is enabled. - if -1: all debug for this debug type is disabled - if >0: all debug that is < to the level is enabled. This means that when checking which level is enabled, 0 and 1 are identical. The LDBG() macro is updated to accept an optional log level to illustrate the feature. Here is the expected behavior: LDBG() << "A"; LDBG(2) << "B"; With `--debug-only=some_type`: we'll see A and B in the output. With `--debug-only=some_type:-1`: we'll see neither A not B in the output. With `--debug-only=some_type:1`: we'll see A but not B in the output. With `--debug-only=some_type:2`: we'll see A and B in the output. (same with any level above 2) Note that LDBG() is equivalent to LDBG(0) which is equivalent to LDBG(1).
During the review of llvm#150855 we switched from 0 to 1 for the default log level used, but this macro wasn't updated.
During the review of #150855 we switched from 0 to 1 for the default log level used, but this macro wasn't updated.
This allows to set an optional integer level for a given debug type. The string format is
type[:level], and the integer is interpreted as such:The LDBG() macro is updated to accept an optional log level to illustrate the feature. Here is the expected behavior:
LDBG() << "A"; // Identical to LDBG(1) << "A";
LDBG(2) << "B";
With
--debug-only=some_type: we'll see A and B in the output.With
--debug-only=some_type:1: we'll see A but not B in the output.With
--debug-only=some_type:2: we'll see A and B in the output. (same with any level above 2)With
--debug-only=some_type:0: we'll see neither A nor B in the output, but we'll see any other logging for other debug types.